Skip to content

Conversation

@UsmanNadeem
Copy link
Contributor

@UsmanNadeem UsmanNadeem commented Nov 22, 2025

Reapply: #157646 by reverting: #167352

@UsmanNadeem
Copy link
Contributor Author

@boomanaiden154 Fix for the existing reported issue has been merged. Can you see if this pass is causing any more breakage for you?

Copy link
Member

@XChy XChy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@boomanaiden154
Copy link
Contributor

I started testing. I think I should have a decent chunk of results by tomorrow. I'll update the thread with results once I have them.

@boomanaiden154
Copy link
Contributor

I am seeing some failures that look like miscompiles still. No crashes from what I've seen yet, and there were one or two before.

I'll try and spend some time debugging tomorrow, along with digging through the rest of the results. The one failure I've seen so far is in https://github.com/google/xls in partial_ops_test if anyone else wants to try and take a stab at it before I get to it.

@boomanaiden154
Copy link
Contributor

https://gist.github.com/boomanaiden154/445779ab0e9bd122b9c8c15ce00db48f

Reproduces the issue:

aidengrossman@aidengrossman:/work/llvm-project/build$ ./bin/clang++ --target=x86_64 -Wno-override-module -O3 ./seed-renamed.ll -mllvm -enable-dfa-jump-thread=true -o /tmp/dfa_jump_thread
aidengrossman@aidengrossman:/work/llvm-project/build$ /tmp/dfa_jump_thread
aidengrossman@aidengrossman:/work/llvm-project/build$ echo $?
1
aidengrossman@aidengrossman:/work/llvm-project/build$ ./bin/clang++ --target=x86_64 -Wno-override-module -O3 ./seed-renamed.ll -mllvm -enable-dfa-jump-thread=false -o /tmp/normal
aidengrossman@aidengrossman:/work/llvm-project/build$ /tmp/normal
aidengrossman@aidengrossman:/work/llvm-project/build$ echo $?
0
aidengrossman@aidengrossman:/work/llvm-project/build$ git log --oneline --max-count=1
e71f243a8d0e (HEAD -> main, origin/main, origin/HEAD) [TableGen] Simplify MachineValueTypeSet::iterator::find_from_pos. NFC (#169227)

Sorry for the largeish reproducer. llvm-reduce kept reducing the test case in such a way that the interestingness test began to fail.

@XChy
Copy link
Member

XChy commented Nov 25, 2025

https://gist.github.com/boomanaiden154/445779ab0e9bd122b9c8c15ce00db48f

I tried to reduce it with https://github.com/dtcxzyw/llvm-ub-aware-interpreter, but it contains immediate UB:

UB triggered: load from poison pointer
Exited with immediate UB.
Stacktrace:
    %load = load ptr, ptr poison, align 8 at @hoge.107
    call void @hoge.107() #7 at @pluto
    call void @pluto(ptr noundef nonnull align 8 dereferenceable(24) %alloca1, ptr %load, i64 %load16) at @main

@boomanaiden154
Copy link
Contributor

I think that's just how llvm-reduce does things. quux is the function that contains the bad transformation (not sure how things look after inlining).

@XChy
Copy link
Member

XChy commented Nov 25, 2025

Do you have the original unreduced IR file? I am going to try to reduce a UB-free testcase.

@boomanaiden154
Copy link
Contributor

Do you have the original unreduced IR file? I am going to try to reduce a UB-free testcase.

Not one I can just post here given this comes from our internal build system..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants